-
Notifications
You must be signed in to change notification settings - Fork 6.8k
ci: setup automatic deployment for the docs app #24528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Note: As mentioned in the "proposal doc", moving the docs repo into |
11dc3bf
to
4c77c55
Compare
I chatted about this to a good extent with @gkalpak. Long-term we certainly want to share the same script from AIO, and potentially also use redirects (e.g. where
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
36ebd37
to
182c8ec
Compare
@josephperrott addressed your feedback and also tested on my fork again. We'll also need a patch-port |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall lgtm (with a few comments) expect for the test values (e.g. for branch names, Firebase project/site IDs, remote URLs, etc.) that need to be updated.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
182c8ec
to
d6c193e
Compare
98767b0
to
1bddbc7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
1bddbc7
to
0f2fd13
Compare
Note: Corresponding patch PR that needs to go in once this gets merged: #24552 |
13c1a84
to
71b586e
Compare
See the following document for the overall plan that this commit implements: https://docs.google.com/document/d/1xkrSOFa6WeFqyg1cTwMhl_wB8ygbVwdSxr3K2-cps14/edit
We currently configure RBE by setting `GOOGLE_APPLICATION_CREDENTIALS` into the `$BASH_ENV` variable, ensuring RBE is configured everywhere on CI. This worked nicely but now with automatic docs deployment turned out to be problematic since it prevents scripts from defining `GOOGLE_APPLICATION_CREDENTIALS` themselves/overriding it. the reason is that `$BASH_ENV` always runs in new child processes (like when firebase is initialized) and then overrides the credentials back to the RBE service key. We can simplify this code by using a dedicated Bazel flag.
71b586e
to
a59977e
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
See the following document for the overall plan that this
commit implements:
https://docs.google.com/document/d/1xkrSOFa6WeFqyg1cTwMhl_wB8ygbVwdSxr3K2-cps14/edit
We will use a test firebase project temporarily to ensure its working as intended.